PID, Drivetrain, cleanup, other little things#41
PID, Drivetrain, cleanup, other little things#41lhmcgann merged 9 commits intoDeepBlueRobotics:masterfrom
Conversation
implemented velocity PID controllers: class, RobotMap, DT centralized move and turn PID controllers in respective commands --> deleted them from RobotMap and their methods from Drivetrain made some more things InstantCommands tests for velocity controllers and new PIDSourceAverage class no tests for PIDMove or PIDTurn commands bc don't know how to get around need for SmartDashboard/NetworkTables deleted gyro stuff bc we don't need/use it: have the ahrs deleted LeftDrive and RightDrive classes and references in Robot general cleanup of Drivetrain and its interface
https://github.com/DeepBlueRobotics/RobotCode2018.git Conflicts: Robot2018/src/org/usfirst/frc/team199/Robot2018/RobotMap.java Robot2018/src/org/usfirst/frc/team199/Robot2018/autonomous/VelocityPIDController.java hopefully this resolves the conflicts w/ Kate's merge :/
| leftEncDist.setPIDSourceType(PIDSourceType.kDisplacement); | ||
| leftEncRate = new Encoder(leftEncPort1, leftEncPort2); | ||
| leftEncRate.setPIDSourceType(PIDSourceType.kRate); | ||
| dtLeftDrive = new WPI_TalonSRX(getPort("LeftTalonSRXDrive", 0)); |
There was a problem hiding this comment.
I recommend changing the names of dtLeftDrive and dtRightDrive (and the associated pref names) to use the term "master" instead of "drive" to avoid confusion with the dtLeft and dtRight. Also, if there isn't a good reason to make the master and slave motor controller variables public, they should be private.
There was a problem hiding this comment.
I will probably be reorganizing the whole component instantiation setup, trying to remove statics and push everything possible into respective subsystems. So i'm going to leave it for now but expect to change the whole thing soon (see issue #49)
| leftVelocityController.setInputRange(0, Double.MAX_VALUE); | ||
| leftVelocityController.setOutputRange(-1.0, 1.0); | ||
| leftVelocityController.setContinuous(false); | ||
| leftVelocityController.setAbsoluteTolerance(Robot.getConst("ConstMoveToleranceLeft", 2)); |
There was a problem hiding this comment.
Shouldn't the above uses of the word "move" be changed to "velocity"?
| * @param output | ||
| * the SpeedController you are telling what to do | ||
| */ | ||
| public VelocityPIDController(double kp, double ki, double kd, PIDSource source, SpeedControllerGroup output) { |
There was a problem hiding this comment.
Should also pass in kf, or even better, the info needed to compute kf.
| } | ||
| assertEquals(avg.getPIDSourceType(), PIDSourceType.kRate); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please add at least one test that verifies that it does actually return the average of the 2 sources.
bc .gitignore stuff from testing-no-auto
added f in VelocityPIDController constructors to resolve errors added test to PIDSourceAverageTest to make sure it actually returns the average
per Corvin's suggestion in order to clarify and leave no room for confusion about what is what (e.g. button was called PIDMove but so was the command it was calling; changed to PIDMoveButton, etc.) yeah, I know the names are long, but you know exactly what they are :)
all in RobotMap changed TalonSRX dt motor controller names from Drive -> Master per request by Dean forgot to delete actual AnalogGyro in RobotMap, so did that
|
@brettle Do you know the purpose/use of the kOff DoubleSolenoid.Value? I know it sets the solenoid to neutral but I was unsure why/when it would be used. I asked bc in this PR I changed shifting gears commands to InstantCommands, but @doawelul had commented in issue #31 about not doing so in order to set the solenoid to kOff after shifting. I tried looking online but it wasn't very helpful, so I was wondering if you knew? --> Is holding in KForward or kReverse ok? --> Is shifting directly from kForward to kReverse and back ok? --> Is changing the shifting gear to InstantCommands ok? |
|
Holding kForward or kReverse should be fine. The solenoid should only draw a small fraction of an amp. |
|
Ok, cool, thank you. 👍 |
2 participants
deleted them from RobotMap and their methods from Drivetrain
need for SmartDashboard/NetworkTables